PML/UCX: improved error processing in MPI_Recv#8140
Merged
yosefe merged 1 commit intoopen-mpi:masterfrom Nov 3, 2020
Merged
Conversation
brminich
approved these changes
Oct 28, 2020
yosefe
reviewed
Oct 28, 2020
ompi/mca/pml/ucx/pml_ucx.c
Outdated
| status = ucp_request_test(req, &info); | ||
| if (status != UCS_INPROGRESS) { | ||
| mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); | ||
| mca_pml_ucx_set_recv_status(mpi_status, status, &info); |
Contributor
There was a problem hiding this comment.
maybe add return value for mca_pml_ucx_set_recv_status(), instead of extra branch in lines 615-616?
Contributor
Author
There was a problem hiding this comment.
branch on 615-616 needed to call mca_pml_ucx_set_recv_status. I can move it in to mca_pml_ucx_set_recv_status_safe
Contributor
Author
|
@yosefe ok to squash? |
yosefe
reviewed
Oct 29, 2020
ompi/mca/pml/ucx/pml_ucx_request.h
Outdated
Comment on lines
197
to
201
| ompi_status_public_t _local_status; | ||
| ompi_status_public_t *mpi_status = (_mpi_status != MPI_STATUS_IGNORE) ? | ||
| _mpi_status : &_local_status; | ||
|
|
||
| return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); |
Contributor
There was a problem hiding this comment.
maybe avoid setting temp mpi_status field if not needed
if (mpi_status != MPI_STATUS_IGNORE) {
return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info)
} else if (status == UCS_OK || status == CANCELED) {
return UCS_OK;
} else if (status == TRUNCATED)
reutrn ERR_TRUNCATE
} else {
return ERR_INTERN
}
| status = ucp_request_test(req, &info); | ||
| if (status != UCS_INPROGRESS) { | ||
| mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); | ||
| result = mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); |
Contributor
There was a problem hiding this comment.
do we need this also in other places mca_pml_ucx_set_recv_status_safe is used?
Contributor
Author
There was a problem hiding this comment.
fixed for MPI_Mrecv call, other locations are non-blocking MPI calls - not relevant
72cde39 to
4ddfab2
Compare
Contributor
Author
|
@yosefe ok to squash? |
yosefe
reviewed
Nov 1, 2020
ompi/mca/pml/ucx/pml_ucx_request.h
Outdated
| if (mpi_status != MPI_STATUS_IGNORE) { | ||
| mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); | ||
| return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); | ||
| } else if ((ucp_status == UCS_OK) || (ucp_status == UCS_ERR_CANCELED)) { |
2946abd to
4ef5fc2
Compare
Contributor
Author
|
@yosefe ok to squash? |
- improved error processing in MPI_Recv implementation of pml UCX - added error handling for pml_ucx_mrecv call Signed-off-by: Sergey Oblomov <sergeyo@nvidia.com>
4ef5fc2 to
eb9405d
Compare
Contributor
Author
|
@yosefe ok to merge? |
yosefe
approved these changes
Nov 3, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
of pml UCX
Signed-off-by: Sergey Oblomov sergeyo@mellanox.com